chroot: handle slightly broken seccomp defaults#2105
chroot: handle slightly broken seccomp defaults#2105nalind wants to merge 1 commit intocontainers:masterfrom
Conversation
When a seccomp rule includes multiple equality checks for the same argument for a syscall, they can never ALL be satisfied. Because that's how they're supposed to be treated, libseccomp returns an error when we try to add them as part of the same conditional rule. Try to detect this exact case, and if we detect it, treat each condition as its own rule. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
| // were OR'd, when in fact they're ordinarily | ||
| // supposed to be AND'd. Break them up into | ||
| // different rules to get that OR effect. | ||
| if len(rule.Args) > 1 && opsAreAllEquality && err.Error() == "two checks on same syscall argument" { |
There was a problem hiding this comment.
Where's the string from? It always worries me to check for an error by looking at the string output rather than a particular error type.
There was a problem hiding this comment.
I share your concern. This is an error object that github.com/seccomp/libseccomp-golang creates every time it returns the error, so we can't just compare the error itself for equality with a known value. If there's a better method, I'm happy to switch to it.
TomSweeneyRedHat
left a comment
TomSweeneyRedHat
left a comment
There was a problem hiding this comment.
LGTM
@rhatdan @vrothberg PTAL
vrothberg
left a comment
vrothberg
left a comment
There was a problem hiding this comment.
I have a hard time reviewing this. Is there any chance we can generalize the problem and get it into libseccomp-golang?
|
📌 Commit f4f2c05 has been approved by |
|
☀️ Test successful - status-papr, status-travis |
Why was it merged without a reply to my concerns above? |
|
I saw this as a separate PR, not necessarily related to Buildah. I am also wondering if the chroot code is specific to Buildah, and not sure if this is useful to other projects. |
|
FWIW, we're doing this here instead of the seccomp bindings to mirror part of how runc does things in opencontainers/runc#1616. |
5 participants
When a seccomp rule includes multiple equality checks for the same argument for a syscall, they can never ALL be satisfied. Because that's how they're supposed to be treated, libseccomp returns an error when we try to add them as part of the same conditional rule. Try to detect this exact case, and if we detect it, treat each condition as its own rule.